Specify that Reader::read doesn't return 0. #18289
Conversation
|
Once this has been merged, some Reader functions can be simplified. E.g., read_u8 currently calls a large function (read_at_least) which then repeatedly calls read (up to 1000 times if read returns 0.) This might be the reason that read_u8 is significantly slower than read(buf) where buf has size 1. |
Reader is a trait for byte-oriented streams. Unlike with packet oriented protocols such as UDP or lower level network traffic, there is no reason for a read on a stream to ever return a zero bytes buffer. If data is available, read shall return a suitable amount of it in the buffer. If no data is available, read shall either block or signal a condition via an Error. Note that it is already impossible to use many Reader methods on packet oriented protocols. E.g., consider reading an u64. 1) If a new packet is available, what should be done with the remainder of the packet? 2) If we've stored a previous packet and there are still four bytes available, should we a) attempt to read another packet and possibly block and then combine the bytes into an 8 byte sequence, or b) return an error? Objects that implement packet oriented protocols or for which it makes sense to return 0-sized reads should implement read methods that are independent of the Reader trait. This is a breaking change because there might be Reader implementations that return 0, however, no such implementations are in the standard library. [breaking-change]
|
I forgot about the case where the buffer is empty. I think it's best to leave this case up to the implementation. |
| /// # Error | ||
| /// # Return value | ||
| /// | ||
| /// If the length of `buf` is 0, the return value is unspecified. |
There was a problem hiding this comment.
Can this be changed to say that implementations may either return Ok(0) or Err(...) at their choosing? It seems like that shouldn't effectively limit implementations and it'll be good to forbid totally silly values like Ok(1123465234).
There was a problem hiding this comment.
Do you have cases in mind where returning Ok(1) would be dangerous?
There was a problem hiding this comment.
Methods that pass a buffer to read and then slice it down to the part that was filled will panic, for example.
|
Even after this, I think splitting Reader up into a basic trait and a specialized trait for blocking readers is inevitable. The documentation of Reader already makes a passing remark that
but this should not be required in general and is not true in today's rust programs, even in the stdlib. E.g., there is a long issue talking about stdout being non-blocking in some cases and how this case should be handled (libnative currently doesn't handle this case.) The The following methods can theoretically work fine with non-blocking read:
The rest will not work. Consider a reader implemented on top of a large but finite non-blocking byte stream.
In general, everything that might call How should this be handled? Some ideas:
A basic implementation of the third version including optimized versions of |
|
The worry of non-blocking reads has been brought up in the past, and it definitely seems similar to the UDP where it's packet-oriented instead of byte-oriented. I'm somewhat wary to bolt on this "return of 0 is undefined" behavior clause to For All in all, I do understand that the story of |
|
@mahkoh @alexcrichton any update on this? It's been sitting for almost a month now. |
|
@cmr thanks for the reminder. I think that I'm going to close this out of inactivity, and I think that this will definitely come up when we get around to stabilizing |
…nicola Fix panic when json project has relative buildfile paths The `build_file` path may be relative to the workspace root.
Reader is a trait for byte-oriented streams. Unlike with packet oriented
protocols such as UDP or lower level network traffic, there is no reason
for a read on a stream to ever return a zero bytes buffer.
If data is available, read shall return a suitable amount of it in the
buffer. If no data is available, read shall either block or signal a
condition via an Error.
Note that it is already impossible to use many Reader methods on packet
oriented protocols. E.g., consider reading an u64.
of the packet?
available, should we a) attempt to read another packet and possibly
block and then combine the bytes into an 8 byte sequence, or b) return
an error?
Objects that implement packet oriented protocols or for which it makes
sense to return 0-sized reads should implement read methods that are
independent of the Reader trait.
This is a breaking change because there might be Reader implementations
that return 0, however, no such implementations are in the standard
library.
[breaking-change]
Note that the last paragraph is AFAIK true after #18130
Closes #18079